TYP: require _update_inplace gets Frame/Series, never BlockManager #33074
Conversation
jbrockmendel
changed the title
| @@ -1505,18 +1504,14 @@ def factorize(self, sort=False, na_sentinel=-1): | |||
| def searchsorted(self, value, side="left", sorter=None) -> np.ndarray: | |||
| return algorithms.searchsorted(self._values, value, side=side, sorter=sorter) | |||
|
|
|||
| def drop_duplicates(self, keep="first", inplace=False): | |||
| inplace = validate_bool_kwarg(inplace, "inplace") | |||
| def drop_duplicates(self, keep="first"): | |||
There was a problem hiding this comment.
this is only used in Index I think?
There was a problem hiding this comment.
It exists on Series too
| @@ -1809,7 +1805,7 @@ def unique(self): | |||
| result = super().unique() | |||
| return result | |||
|
|
|||
| def drop_duplicates(self, keep="first", inplace=False) -> "Series": | |||
| def drop_duplicates(self, keep="first", inplace=False) -> Optional["Series"]: | |||
There was a problem hiding this comment.
are we actually testing the return value of inplace ops for drop_duplicates / duplciated? IIRC these are actually returning an object (at least they were).
There was a problem hiding this comment.
are we actually testing the return value of inplace ops for drop_duplicates / duplciated?
Yes, editing this to return self instead of None breaks a test in tests.frame.test_api
…n-update_inplace
|
ok shouldn't you change the doc-string / annotation of _update_inplace (and maybe add an assertion) to guarantee this? |
Tricky since we could be dealing with a subclass. I think its pretty clear in context. |
|
are remaining comments deal-breakers? nice follow-ups to de-duplicate some code after this |
|
no my point on the _update_inplace method should only accept Series/Dataframe and not a BlockManager anymore i think that’s reasonable to add |
Right, thats what this PR does. The docstring is updated to reflect that. Just havent added an annotation because we dont have a ready-made "same type as self" |
|
k fair enough maybe an assertion would be good at some point |
No description provided.